Optimize: Keep cont->mutex locked, during probe the bucket by hash object within HostDBProcessor::getby#4916
Conversation
iocore/hostdb/HostDB.cc
Outdated
| loop = false; // Only loop on explicit set for retry. | ||
| // find the partition lock | ||
| // | ||
| // TODO: Could we reuse the "mutex" above safely? I think so but not sure. |
There was a problem hiding this comment.
Should delete this comment, it's clearly not safe because the lock on line 619 is scoped and has been dropped at this point.
iocore/hostdb/HostDB.cc
Outdated
| if (!aforce_dns) { | ||
| MUTEX_TRY_LOCK(lock, cont->mutex, thread); | ||
| if (!lock.is_locked()) { | ||
| goto Ldelay; |
There was a problem hiding this comment.
I'm very much not fond of goto. In this case, I'd be tempted to change the do;while(loop) into a while (loop). E.g. something like
MUTEX_TRY_LOCK(...);
bool loop = lock.is_locked();
while (loop) {
loop = false; // ...
// ...
}
There was a problem hiding this comment.
Updated per your suggestion.
oknet
force-pushed
the
i4916
branch
2 times, most recently
from
613d359 to
784caa6
Compare
|
Could it be dead lock ? See trafficserver/iocore/hostdb/HostDB.cc Lines 668 to 683 in 784caa6 I'm not sure here, But the HostDBContinuation use the same bucket lock. And it grab the bucket lock first then get continuation's lock (which equals to action.mutex). I know that the the cont will run after |
|
I think not. The locking is try, not hard locks, which prevents deadlock. Any continuation that gets one lock but not the other will drop the acquired lock rather than waiting on the second lock. |
…ject within HostDBProcessor::getby
|
Rebased to resolve merge conflicts with #4926. |
3 participants
No description provided.